Skip to content

Conversation

@riking
Copy link

@riking riking commented Sep 2, 2014

This allows consumers to check whether or not Github has verified the emails, or use non-primary email addresses if they want. Important for consumers that need validated emails.

Additionally, a new 'display_email' field is added to info, which will always be the email displayed on your GitHub user page.

Fixes #36

@riking riking changed the title Include all email data in extra response Include all email data in 'extra' response Sep 2, 2014
This allows consumers to check whether or not Github has verified the
emails.

Manual selection by consumers is also available using the emails array
in the `extra` response data.

Additionally, a new 'display_email' field is added to `info`, which will
always be the email displayed on your GitHub user page.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes nil values to be changed into false.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usual approach is !!(something)

@SamSaffron
Copy link

Note, this is real urgent for Discourse, I am forking for now till its upstreamed.

@SamSaffron
Copy link

ping @mbleigh this is a pretty severe

@mbleigh
Copy link
Contributor

mbleigh commented Sep 10, 2014

I think that a better solution for this might be to put the non-verified email in an unverified_email key. By adding a second key we are implicitly requiring users to make a check to ensure security.

A separate key containing an unverified_email would have to be explicitly accessed and pose less of a risk. Thoughts?

@SamSaffron
Copy link

I agree with that, but it is a bit of a "good" breaking change. I think all the emails omniauth-github deals out should be verified. People should need to do extra work to get an unverified one.

@mbleigh
Copy link
Contributor

mbleigh commented Sep 10, 2014

@SamSaffron I think we're saying the same thing. I'm saying that a boolean email_verified flag is not as secure as making unverified emails only accessible through a separate key on the info hash.

And therefore we should do the more secure thing.

@mbleigh
Copy link
Contributor

mbleigh commented Feb 13, 2015

I'd still accept this if it were changed to put an unverified email in an unverified_email key instead of the email key.

@riking
Copy link
Author

riking commented May 11, 2015

Merged in #48

@riking riking closed this May 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ominauth github returns unverified email addresses

3 participants